In "cargo install" directly from registry, don't require optional dependencies

When building with a directory registry that contains only the subset of
crates required to build an application crate, cargo fails if that
subset doesn't include optional dependencies pulled in for every
possible feature of the root crate, even when the install doesn't enable
those features.  This prevents Linux distributions from building with
a minimal set of dependencies (omitting, for instance, packages for
unstable/nightly features).

Introduce a new workspace flag "require_optional_deps", disabled for
install and enabled for everything else.  Skip the initial
Method::Everything resolve in this case, and modify
resolve_with_previous to support running a Method::Required resolve
without a previous resolution.

This also skips adding path overrides, as those won't make sense (and
won't work) for an install directly from a registry.

Introduce a set of tests for "cargo install" directly from a directory
registry.

diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
index 8a4cb0165d01e2f78121d5baa403a832e19b5b8f..00fd7da2920abddbb7ee5a8468fa507df4cf6eb6 100644 (file)
--- a/src/cargo/core/workspace.rs
+++ b/src/cargo/core/workspace.rs
@@ -44,6 +44,11 @@ pub struct Workspace<'cfg> {
     // True, if this is a temporary workspace created for the purposes of
     // cargo install or cargo package.
     is_ephemeral: bool,
+
+    // True if this workspace should enforce optional dependencies even when
+    // not needed; false if this workspace should only enforce dependencies
+    // needed by the current configuration (such as in cargo install).
+    require_optional_deps: bool,
 }
 
 // Separate structure for tracking loaded packages (to avoid loading anything
@@ -99,6 +104,7 @@ impl<'cfg> Workspace<'cfg> {
             target_dir: target_dir,
             members: Vec::new(),
             is_ephemeral: false,
+            require_optional_deps: true,
         };
         ws.root_manifest = ws.find_root(manifest_path)?;
         ws.find_members()?;
@@ -115,8 +121,8 @@ impl<'cfg> Workspace<'cfg> {
     ///
     /// This is currently only used in niche situations like `cargo install` or
     /// `cargo package`.
-    pub fn ephemeral(package: Package, config: &'cfg Config, target_dir: Option<Filesystem>)
-                     -> CargoResult<Workspace<'cfg>> {
+    pub fn ephemeral(package: Package, config: &'cfg Config, target_dir: Option<Filesystem>,
+                     require_optional_deps: bool) -> CargoResult<Workspace<'cfg>> {
         let mut ws = Workspace {
             config: config,
             current_manifest: package.manifest_path().to_path_buf(),
@@ -128,6 +134,7 @@ impl<'cfg> Workspace<'cfg> {
             target_dir: None,
             members: Vec::new(),
             is_ephemeral: true,
+            require_optional_deps: require_optional_deps,
         };
         {
             let key = ws.current_manifest.parent().unwrap();
@@ -219,6 +226,10 @@ impl<'cfg> Workspace<'cfg> {
         self.is_ephemeral
     }
 
+    pub fn require_optional_deps(&self) -> bool {
+        self.require_optional_deps
+    }
+
     /// Finds the root of a workspace for the crate whose manifest is located
     /// at `manifest_path`.
     ///

diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs
index 5ad84e376ff5125df637e5f1a46e6f6d84f521b5..fddb3b1ec232f21f5eaa479620871557f720f01f 100644 (file)
--- a/src/cargo/ops/cargo_install.rs
+++ b/src/cargo/ops/cargo_install.rs
@@ -97,7 +97,7 @@ pub fn install(root: Option<&str>,
     };
 
     let ws = match overidden_target_dir {
-        Some(dir) => Workspace::ephemeral(pkg, config, Some(dir))?,
+        Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?,
         None => Workspace::new(pkg.manifest_path(), config)?,
     };
     let pkg = ws.current()?;

diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
index a93fc234ab5f065b6a65309c5939d567f5b381ef..510412c01b94710876e57d4006cddeb17d4446e8 100644 (file)
--- a/src/cargo/ops/cargo_package.rs
+++ b/src/cargo/ops/cargo_package.rs
@@ -284,7 +284,7 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()>
     let new_pkg = Package::new(new_manifest, &manifest_path);
 
     // Now that we've rewritten all our path dependencies, compile it!
-    let ws = Workspace::ephemeral(new_pkg, config, None)?;
+    let ws = Workspace::ephemeral(new_pkg, config, None, true)?;
     ops::compile_ws(&ws, None, &ops::CompileOptions {
         config: config,
         jobs: opts.jobs,

diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
index c4caf63e0d66e5ae929906ed94799e321f9a728f..03825e13e0c646eaff577c7758e55c049212a89a 100644 (file)
--- a/src/cargo/ops/resolve.rs
+++ b/src/cargo/ops/resolve.rs
@@ -37,16 +37,22 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
         registry.add_preloaded(source);
     }
 
-    // First, resolve the root_package's *listed* dependencies, as well as
-    // downloading and updating all remotes and such.
-    let resolve = resolve_with_registry(ws, &mut registry)?;
+    let resolve = if ws.require_optional_deps() {
+        // First, resolve the root_package's *listed* dependencies, as well as
+        // downloading and updating all remotes and such.
+        let resolve = resolve_with_registry(ws, &mut registry)?;
+
+        // Second, resolve with precisely what we're doing. Filter out
+        // transitive dependencies if necessary, specify features, handle
+        // overrides, etc.
+        let _p = profile::start("resolving w/ overrides...");
 
-    // Second, resolve with precisely what we're doing. Filter out
-    // transitive dependencies if necessary, specify features, handle
-    // overrides, etc.
-    let _p = profile::start("resolving w/ overrides...");
+        add_overrides(&mut registry, ws)?;
 
-    add_overrides(&mut registry, ws)?;
+        Some(resolve)
+    } else {
+        None
+    };
 
     let method = if all_features {
         Method::Everything
@@ -60,7 +66,7 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
 
     let resolved_with_overrides =
     ops::resolve_with_previous(&mut registry, ws,
-                               method, Some(&resolve), None,
+                               method, resolve.as_ref(), None,
                                specs)?;
 
     for &(ref replace_spec, _) in ws.root_replace() {
@@ -159,8 +165,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
             // members in the workspace, so propagate the `Method::Everything`.
             Method::Everything => Method::Everything,
 
-            // If we're not resolving everything though then the workspace is
-            // already resolved and now we're drilling down from that to the
+            // If we're not resolving everything though then we're constructing the
             // exact crate graph we're going to build. Here we don't necessarily
             // want to keep around all workspace crates as they may not all be
             // built/tested.
@@ -176,7 +181,6 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
             // base method with no features specified but using default features
             // for any other packages specified with `-p`.
             Method::Required { dev_deps, .. } => {
-                assert!(previous.is_some());
                 let base = Method::Required {
                     dev_deps: dev_deps,
                     features: &[],

diff --git a/tests/directory.rs b/tests/directory.rs
index 9d6dcf1477ff5daac42226f62913b5020a8a49dc..433d6eb56ad62b5d48400c68b010bc34618b6b91 100644 (file)
--- a/tests/directory.rs
+++ b/tests/directory.rs
@@ -10,6 +10,7 @@ use std::str;
 
 use rustc_serialize::json;
 
+use cargotest::cargo_process;
 use cargotest::support::{project, execs, ProjectBuilder};
 use cargotest::support::paths;
 use cargotest::support::registry::{Package, cksum};
@@ -105,6 +106,144 @@ fn simple() {
 "));
 }
 
+#[test]
+fn simple_install() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(0).with_stderr(
+"  Installing bar v0.1.0
+   Compiling foo v0.1.0
+   Compiling bar v0.1.0
+    Finished release [optimized] target(s) in [..] secs
+  Installing [..]bar[..]
+warning: be sure to add `[..]` to your PATH to be able to run the installed binaries
+"));
+}
+
+#[test]
+fn simple_install_fail() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+            baz = "9.8.7"
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(101).with_stderr(
+"  Installing bar v0.1.0
+error: failed to compile `bar v0.1.0`, intermediate artifacts can be found at `[..]`
+
+Caused by:
+  no matching package named `baz` found (required by `bar`)
+location searched: registry https://github.com/rust-lang/crates.io-index
+version required: ^9.8.7
+"));
+}
+
+#[test]
+fn install_without_feature_dep() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+            baz = { version = "9.8.7", optional = true }
+
+            [features]
+            wantbaz = ["baz"]
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(0).with_stderr(
+"  Installing bar v0.1.0
+   Compiling foo v0.1.0
+   Compiling bar v0.1.0
+    Finished release [optimized] target(s) in [..] secs
+  Installing [..]bar[..]
+warning: be sure to add `[..]` to your PATH to be able to run the installed binaries
+"));
+}
+
 #[test]
 fn not_there() {
     setup();